VERILOG CHECKLIST
Coding in Verilog? You're risking going crazy unless you follow these checklist items.
Compilation Units
`default_nettype none. This is unconditional. Patches will be refused if your Verilog modules lack this declaration. Not including this has given me (no joke!) months of grief when debugging.
Prefer parameters over preprocessor directives. (This is placed at the module level since preprocessor definitions are global in scope.) Are they really necessary? Prefer parameters and generate blocks over preprocessor directives when and where possible. They take less typing to use on-average, which means fewer mistakes.
module
Headers
All signals are listed without qualification. For example, it's not
input foo
oroutput [63:0] bar
, it's justfoo
orbar
.There is a comma separating every signal name.
There is not a comma after the last signal name.
If you do not employ the Haskell-style module declaration (see below), list
i_clk
andi_reset
as the last signals in the module. If you do employ the Haskell-style module declaration method, listi_clk
andi_reset
as the first signals in the module. You'll never have to change these signals, so if you need to rearrange signal groupings later on, you can do so without concern for the separating commas.All inputs should start with
i_
.All outputs should start with
o_
.All bidirectional signals should be split into a distinct input and output, along with an output enable output to indicate data direction to external logic or I/O blocks.
Groups of related signals (as with a bus or single port) should have its mnemonic name follow the
i_
oro_
prefix. For example,i_master_valid
oro_slave_ready
. Never usemaster_i_valid
, or the like. Deprecate formats likemaster_valid_i
.All signal names should be spelled with lowercase characters. Uppercase names are deprecated.
All parameters should be in all upper-case notation, just as preprocessor symbols would be. They serve a similar purpose as preprocessor macros, so their nomenclature should resemble this.
Parameters that cannot be changed by the instantiating module, but are good practice to name anyway, should be
localparam
s
NOTE: A lot of my own Verilog sources will break at least one of these rules. That's because I've not suffered pain from violating these rules at the time I wrote them. They will be modernized over time on an as-needed basis.
BAD
module FIFO(
input i_reset,
input i_clk,
input i_a_valid,
output i_a_ready,
input [`UPB:0] i_a_byte,
output o_b_valid,
input i_b_ready,
output [`UPB:0] o_b_byte
);
GOOD
module FIFO(
i_a_valid,
i_a_ready,
i_a_byte,
o_b_valid,
i_b_ready,
o_b_byte,
i_reset,
i_clk
);
parameter BUS_WIDTH = 8;
localparam UPB = (BUS_WIDTH-1);
input wire [UPB:0] i_a_byte;
output wire [UPB:0] o_b_byte;
reg [UPB:0] r_b_byte;
assign o_b_byte = r_b_byte;
ALSO GOOD
I call this the Haskell-influenced module header. Haskell programmers will be able to readily identify why.
module FIFO(
i_reset
, i_clk
, i_a_valid
, i_a_ready
, i_a_byte
, o_b_valid
, i_b_ready
, o_b_byte
);
parameter BUS_WIDTH = 8;
localparam UPB = (BUS_WIDTH-1);
input wire [UPB:0] i_a_byte;
output wire [UPB:0] o_b_byte;
reg [UPB:0] r_b_byte;
assign o_b_byte = r_b_byte;
WHY
Bluntly, it's because literally nobody can agree on a common subset of available Verilog syntaxes when it comes to module declarations, and I'm frankly tired of dealing with it. As they say in the Python community, explicit is better than implicit.
Of course, at some point, you'll need to qualify the module signals. That leads us to . . .
Signals and Assignments
Some of these are merely stylistic issues. I don't necessarily agree with all of them, but in the interest of supporting a burgeoning open-hardware community, I'd rather a consistent coding convention be used to facilitate easier code sharing.
Always use 0 for a lower bound on a bus. If you really want to use a non-zero lower-bound, make sure assignments are fully qualified. ZipCPU puts it best, I think: I recently got burned with a "wire [22:5] value;" declaration. Setting value to 1 gave me some problems, as Yosys tried to synthesize a bit [0]. I normally always use a 0 LSB, but in this case the [22:5] made a lot of things easier to read. Hence, I'd suggest preferring a zero LSB or (if not) always specifying which bits you are talking about. value[22:5]=1 worked, value = 1 did not.
For blocking assignments, always use
always @(*)
. Again, from ZipCPU's storied experience: I use blocking assignments only in always @() blocks--keeps Verilator from complaining about them.*Avoid
assign
s when using?:
operator. ZipCPU writes: If I have more than one ?: operator, I try to avoid assign's, choosing instead to use an always @()* (todo -- find out why this is.)Avoid
initial
blocks. They are a great aid for simulation, but have variable semantics across vendors of (and, sometimes, families offered by a single vendor of) FPGAs and aren't supported at all by ASIC workflows. Replace with explicit reset logic if their value is important. Note that these are different from initializers. Please see below for more details about initializers.
Output Signals
All outputs are wires. Unconditionally. Forever and ever. Do not trust
output reg
.If an output must be a register, instantiate it manually.
NEVER use an initializer with an
output
statement.Qualify and declare your outputs next to the process that is responsible for driving them.
NOTE: It's technically valid Verilog to write an initializer as long as it is a constant expression. However, for the sake of uniformity and reducing the amount of special-cases the developer has to be concerned with, *just don't.
If an output X
is to be driven by a register,
then you should have the following declarations in the source file:
reg r_X;
output wire o_X;
assign o_X = r_X;
If X
is a bus:
reg [UB:LB] r_X;
output wire [UB:LB] o_X;
assign o_X[UB:LB] = r_X;
If a signal is hard-wired to a specific value:
output wire [2:0] o_foo;
assign o_foo = 3'b101;
BAD
module foo(
// ...
output o_bar
);
// 1500 lines of gibberish
always @(posedge i_clk) begin
// 150 more lines of irrelevant code
if(some_condition) begin
o_bar <= ...etc...;
end
end
endmodule;
GOOD
module foo(
// ...
o_bar,
// ...
)
// 1500 lines of gibberish
output wire o_bar;
reg r_bar;
assign o_bar = r_bar;
always @(posedge i_clk) begin
if(i_reset) r_bar <= `BAR_RESET_VALUE;
else if(some_condition) r_bar <= `SOME_OTHER_VALUE;
else ...
end
// etc...
endmodule
WHY
This is damn annoying to have to do,
but it's been a major source of heart-ache when trying to unify
the results of formal verification against testing tools like Verilator.
It seems nobody can detect illegal variations of the
output X = Y;
statement (where Y is not a constant).
Therefore, avoid them all-together. Don't even look down that dark alley.
(I started to get into this bad habit because of Xilinx tools which not only accept but also encourages this style of coding.)
Moreover, colocating the registers, assignments, and state transition logic has made things easier to:
Document. I find that my documentation about the effects of a piece of state tends to cluster around a single spot in the source code, instead of being scattered about the source.
Easier to widen/narrow. Signals can often become buses, buses can often widen, or buses can often become narrower depending on your development trajectory.
Easier to just delete everything. It's quite satisfying when removing dead code that is no longer relevant.
Input Signals
If there's no better place to declare a signal as an input, declare it immediately after the
module
statement.If an input is going to be consumed by only one chunk of code, it's best to declare it immediately ahead of that code.